Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui5-dynamic-page): add responsive paddings #7846

Closed
wants to merge 3 commits into from

Conversation

kineticjs
Copy link
Contributor

@kineticjs kineticjs commented Nov 14, 2023

add left/right responsive paddings

@kineticjs kineticjs force-pushed the dp-responsive-margins branch from 82e51fd to 8dafdfb Compare November 20, 2023 08:29
@dobrinyonkov
Copy link
Contributor

dobrinyonkov commented Nov 20, 2023

As we are about to replace all ResizeHandler media range based attributes with container queries: #7666, could we also use them here?

@dobrinyonkov
Copy link
Contributor

Ignore my last comment, we stick to resize handlers for now.

@kineticjs kineticjs marked this pull request as ready for review November 21, 2023 09:05

/* Responsive paddings */

:host([media-range="S"]) .ui5-dynamic-page-root {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should set paddings to all components of the dynamic page individually - title, header, content. This way the background and the box-shadow of the title will be visible through the whole width of the page. Also the footer seems to not need paddings as well.

Notice that I've changed the body background here:
image

@kineticjs kineticjs force-pushed the dp-responsive-margins branch from dcec831 to 6453f54 Compare November 27, 2023 08:11
@kineticjs kineticjs force-pushed the dp-responsive-margins branch from 6453f54 to c143204 Compare November 27, 2023 11:21
@kineticjs kineticjs closed this Nov 27, 2023
@kineticjs
Copy link
Contributor Author

merge issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants